-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DataGrid] Allow to copy the selected rows to the clipboard #1929
Conversation
Is it related to #1197? I had listed a bunch of issues we could solve in isolation. For instance, CTRL+c that triggers the edit mode? No. Here you go, one good PR: alphabetical characters shouldn't trigger the edit mode if ctrl, meta, or shift are pressed 😁. |
@oliviertassinari Interesting, this issue was never mentioned in #199. This PR is actually an enhancement / bugfix of the current copy feature. If you CTRL + c in a row it selects the text then uses the copy command to write to the clipboard, this is not good. I'm leveraging the Clipboard API now. The old copy doesn't support copying the selected rows, now it does and the fields are exported separated by TAB so it can directly be pasted into Excel. I added support for pressing ALT + c to copy including the headers. From the issues you mentioned in #1197:
Fixed.
Fixed.
a. The CSV export is used internatelly, so value formatters are supported |
GRID_KEYDOWN, | ||
getHandler(GRID_KEYDOWN), | ||
); | ||
|
||
React.useEffect(() => { | ||
if (apiRef.current.rootElementRef?.current) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effect doesn't work. When it runs, apiRef.current.rootElementRef?.current
is null. It's already to be removed by #1862, so a native event listener is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start, much better than what we had before 👍
- If the row selection is empty, should CTRL + c copy the focused cell?
- If text is selected natively, inside a read cell, or inside an edit cell, the native CTRL + c behavior is overridden, not great.
- I froze my laptop doing a CTRL + c on the 3m cells demo 😆. It froze Google Spreadsheet too. Only Excel got almost decent performance. AG Grid completely goes sideway when it happens, even with the select-all feature 🙈.
packages/grid/_modules_/grid/hooks/features/clipboard/useGridClipboard.ts
Show resolved
Hide resolved
if (navigator.clipboard) { | ||
navigator.clipboard.writeText(data).catch(() => { | ||
writeToClipboardPolyfill(data); | ||
}); | ||
} else { | ||
writeToClipboardPolyfill(data); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the docs, we use this package: https://github.com/feross/clipboard-copy/blob/master/index.js, in case there is interesting stuff to copy from them. Maybe we could actually make this function in its own file?
packages/grid/_modules_/grid/hooks/features/clipboard/useGridClipboard.ts
Outdated
Show resolved
Hide resolved
const params: GridCellParams = { | ||
...baseParams, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got an improvement from 20ms to 8ms in https://deploy-preview-1929--material-ui-x.netlify.app/components/data-grid/#commercial-version by removing this spread operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, it's pretty wild 👏!
On the full 100,000 rows data set, filtering goes from
https://material-ui.com/components/data-grid/#commercial-version
to
https://deploy-preview-1929--material-ui-x.netlify.app/components/data-grid/#commercial-version
Sorting is unchanged.
packages/grid/_modules_/grid/hooks/features/clipboard/useGridClipboard.ts
Outdated
Show resolved
Hide resolved
packages/grid/_modules_/grid/hooks/features/clipboard/useGridClipboard.ts
Outdated
Show resolved
Hide resolved
packages/grid/_modules_/grid/hooks/features/clipboard/useGridClipboard.ts
Outdated
Show resolved
Hide resolved
…lipboard.ts Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Is there a change in the actual functionality, I mean does the docs needs to be updated? |
@DanailH The actual functionality was not working very well. The docs in general doesn't need updates. Only the accessibility page was updated to mention the new shortcut to copy including headers: Alt + c. |
Related to #199. I didn't use "closes" because this issue is much bigger and we can't really implement it now.
Preview: https://deploy-preview-1929--material-ui-x.netlify.app/components/data-grid/demo/